🟢 Observation: createPkSelectors refactoring changes behavior for nested PK paths in existing APIs

The old private createPkSelectors used StringUtils.substring(pathPart, 1) which treated /address/city as a single segment producing ["address/city"]. The new implementation uses PathParser.getPathParts which correctly splits into ["address"]["city"].

This is a bug fix for nested partition key paths (the old selector looked for a property literally named address/city rather than traversing nested objects), but it changes behavior for two existing code paths:

1. getRangeQueryMap → used by readMany(List<CosmosItemIdentity>)
2. createLogicalPartitionScanQuerySpec → used by readAllItemsOfLogicalPartition

The fix is correct and the risk is low (nested PK paths are uncommon, and the old behavior was wrong), but it may be worth noting in the CHANGELOG since it changes existing readMany behavior.

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

🟡 Remaining: SELECT TOP N queries not rejected by validation

OFFSET and LIMIT were added to the rejection list (fixing the earlier comment), but queryInfo.hasTop() is still missing. SELECT TOP 5 * FROM c would pass validation and the SDK would split it across N physical partitions × M batches, each independently limiting to 5 rows — returning up to 5 × N × M results instead of the expected 5.

hasTop() is available on QueryInfo (line 71 of QueryInfo.java) and is used elsewhere in the codebase (e.g., DocumentQueryExecutionContextFactory:294).

Suggested fix: Add before the hasNonStreamingOrderBy() check:

if (queryInfo.hasTop()) {
    throw new IllegalArgumentException(
        "Custom query for readMany by partition key must not contain TOP.");
}

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

Review Summary: readManyByPartitionKey API

Overall assessment: This is a well-structured, comprehensive addition to the Cosmos SDK. The PR author has been very responsive to feedback — most of the 56 existing review comments have been addressed with fixes.

Key issues resolved since initial review

• ✅ StaleResourceRetryPolicy wrapper added for stale cache resilience
• ✅ PartitionKey.NONE NPE handled via effectivePkInternal fallback
• ✅ End-to-end timeout policy now applied to Spark reader
• ✅ feedResponseProcessedListener diagnostics callback added
• ✅ SQL parser now handles escaped single quotes ('')
• ✅ OFFSET/LIMIT validation added to custom query checks
• ✅ Parameter name collision avoided with @__rmPk_ prefix
• ✅ Batch size default/doc mismatch aligned
• ✅ Null handling made configurable (Null vs None semantics)
• ✅ Empty PK list short-circuit in Spark reader
• ✅ Distributed tracing confirmed via CosmosPagedFlux infrastructure + test coverage

Remaining items (2 new inline comments posted)

1. 🟡 SELECT TOP N not rejected — OFFSET/LIMIT were added but hasTop() is still missing, which could produce semantically incorrect results (per-batch limiting instead of global)
2. 🟢 createPkSelectors refactoring — The move to PathParser.getPathParts fixes nested PK path handling but silently changes behavior for existing readMany and readAllItemsOfLogicalPartition APIs (low risk, but worth a CHANGELOG note)

Architecture highlights

• Good use of round-robin interleaving across physical partitions for concurrent execution
• LinkedHashMap for deterministic iteration order is a nice touch
• The TransientIOErrorsRetryingReadManyByPartitionKeyIterator with page-committed tracking is a solid retry strategy
• PK deduplication via canonical JSON representation handles type coercion correctly

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.